fix: open detail page when clicking edit for time-off policies#1690
Merged
jeffredodd merged 2 commits intomainfrom May 6, 2026
Merged
fix: open detail page when clicking edit for time-off policies#1690jeffredodd merged 2 commits intomainfrom
jeffredodd merged 2 commits intomainfrom
Conversation
jeffredodd
force-pushed
the
fix/non-holiday-policy-guard
branch
from
5a332f0 to
4f26526
Compare
jeffredodd
requested review from
dmortal,
hukid,
krisxcrash and
serikjensen
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes TimeOffFlow navigation so TIME_OFF_VIEW_POLICY correctly opens the policy detail screen for any non-holiday time-off policy type (e.g., custom, bereavement, parental_leave, etc.), instead of being silently ignored due to overly narrow guards/types.
Changes:
- Updated the
TIME_OFF_VIEW_POLICYguard to route all non-holidaypolicy types toviewTimeOffPolicyDetail. - Broadened
TimeOffPolicyTypeto accept the full APIPolicyTypeunion (plus'holiday') so flow context/payloads can represent migrated policy types. - Added parameterized state-machine tests covering several non-holiday policy types and updated the policy-type selector defaulting to only preselect
sick/vacation/holiday.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/components/UNSTABLE_TimeOff/TimeOffFlow/timeOffStateMachine.ts | Broadens view-policy payload typing and updates the view-policy guard to route all non-holiday policies to the standard detail state. |
| src/components/UNSTABLE_TimeOff/TimeOffFlow/timeOffStateMachine.test.ts | Adds coverage to ensure non-holiday (non sick/vacation) policy types transition to viewTimeOffPolicyDetail. |
| src/components/UNSTABLE_TimeOff/TimeOffFlow/timeOffPolicyTypes.ts | Expands TimeOffPolicyType to include all API policy types, enabling flow context to hold migrated values. |
| src/components/UNSTABLE_TimeOff/TimeOffFlow/TimeOffFlowComponents.tsx | Prevents invalid defaults in the create-policy selector by only defaulting when policyType is one of sick/vacation/holiday. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeffredodd
changed the title
The TIME_OFF_VIEW_POLICY transition guarded on policyType === 'sick' || 'vacation', but the API's PolicyType union includes bereavement, custom, floating_holiday, jury_duty, learning_and_development, parental_leave, personal_day, volunteer, and weather. Companies migrated from Gusto.com may have any of these attached to their account. For any non-sick/vacation/holiday type the event matched no guard, the state machine swallowed it, and clicking "Edit policy" in the kabob menu appeared to do nothing. Rename the guard to isNonHolidayView (returns policyType !== 'holiday') so any non-holiday policy routes to viewTimeOffPolicyDetail. Broaden TimeOffPolicyType to PolicyType | 'holiday' so the context can hold the broader values, and narrow back at the create-policy selector site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
serikjensen
approved these changes
May 6, 2026
Restrict edit/add/remove affordances on the policy list and detail screens to sick, vacation, and holiday policies via a shared isEditableTimeOffPolicyType guard. Other policy types (bereavement, custom, parental_leave, etc.) render in view-only mode: the kebab shows "View policy" instead of edit/delete, the detail page hides the add employee, edit policy, and change settings actions, and per-employee balance edit / removal controls are suppressed. Also simplify the holiday pay policy throwOnError to never throw, since it's auxiliary to the main list and any failure is surfaced inline via composeErrorHandler. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jeffredodd
force-pushed
the
fix/non-holiday-policy-guard
branch
from
4f26526 to
798438e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two issues that surfaced when a company has time-off policies outside the supported sick / vacation (PTO) / holiday set — for example, legacy
custom,bereavement, orparental_leavepolicies migrated from Gusto.com:TIME_OFF_VIEW_POLICYtransition guarded onpolicyType === 'sick' || 'vacation', so any other type matched no guard and the event was swallowed. Renames the guard toisNonHolidayView(returnspolicyType !== 'holiday') so any non-holiday policy routes toviewTimeOffPolicyDetail. BroadensTimeOffPolicyTypetoPolicyType | 'holiday'and narrows it back at the create-policy selector (which still only offers sick/vacation/holiday).isEditableTimeOffPolicyTypeguard ('sick' | 'vacation' | 'holiday') used on the policy list and detail screens. Non-editable policies render in view-only mode: the kebab shows "View policy" instead of edit/delete, the detail page hides Add Employee / Edit Policy / Change Settings, and per-employee balance edit and removal are suppressed.Also simplifies the holiday pay policy
throwOnErrorto() => falsesince it's auxiliary to the main list and any failure is surfaced inline viacomposeErrorHandler.Test plan
npm run test -- --run src/components/UNSTABLE_TimeOff/TimeOffFlow/timeOffStateMachine.test.ts— 51/51 passingnpx tsc --noEmit— cleancustom) attached to the company, the policy list kebab shows "View policy" only; opening it shows the detail screen with no edit/add/remove affordances and no balance-edit menu items.🤖 Generated with Claude Code